Add a pull request template providing valuable information when reviewing a PR #15388
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mdelapenya
requested review from
cachedout,
ElWPenn,
kuisathaverat,
mikemadden42,
v1v,
exekias and
andresrc
kuisathaverat
approved these changes
Jan 8, 2020
|
I assume that @urso will want to look at this as well. :) |
cachedout
reviewed
Jan 8, 2020
There was a problem hiding this comment.
The contribution guidelines mention the CLA but I don't see that here. Should it be added?
Yes. This is still in draft to gather the needed information from the team, so any addition is welcome. On the other hand, let's keep the template as concise as possible. |
v1v
approved these changes
Jan 8, 2020
|
jenkins run the tests please |
As this is a one-time check, I would not add it to the PR template. Otherwise it must be removed from the template with each following contribution |
I don't really have strong feelings either way, but it's worth noting that Elasticsearch includes it in their template. |
Me neither, so I'll add it to the checklist, and let's the team decide :) Thanks! |
mdelapenya
commented
Jan 8, 2020
.github/PULL_REQUEST_TEMPLATE.md
Outdated
| - [ ] New and existing **unit** tests pass locally with my changes | ||
| - [ ] New and existing **integration** tests pass locally with my changes | ||
| - [ ] Any dependent changes have been merged and published in downstream modules | ||
| - [ ] New dependencies have been vendorised |
There was a problem hiding this comment.
I'd like the team to discuss about the items in the checklist
There was a problem hiding this comment.
We should add I have made corresponding changes in the default and reference configuration files.
How about thinning out this list as well. Each PR should be run via CI. Only if CI is green shall the PR be merged. The intake job calls make check to validate style (go fmt, autopep8, go vet). Running integration tests requires some time. Although it is appreciated I would not expect devs to run the complete test suite (up to integration tests) upfront.
Items I personally would remove from the list:
- New and existing unit tests pass locally with my changes : the moment we open the PR CI will run tests anyways. Everyone will be able to see if unit tests pass.
- New and existing integration tests pass locally with my changes : Same as unit tests
- New dependencies have been vendorised : The build/CI will fail anyways if that is not the case.
Items I might consider to remove from the list:
- I have signed the : If the CLA checker fails it should print a message + link anyways.
- My changes generate no new warnings. : The go compiler doesn't really generate warnings (it's more oppioninated). And even for C/C++ I'm in favor of using -werror :) . Any other warnings we have in mind?
What exactly is meant by Any dependent changes have been merged and published in downstream modules? Would CI be able to pass if this is not the case?
|
jenkins, test this again please |
urso
reviewed
Jan 9, 2020
.github/PULL_REQUEST_TEMPLATE.md
Outdated
| - [ ] New feature | ||
| - [ ] Bug fix | ||
| - [ ] Enhancement | ||
| - [ ] Breaking change |
There was a problem hiding this comment.
Because Beats is also a framework we do also distinguish between User changes and Developer changes (if some "public" API is changed).
There was a problem hiding this comment.
When you say a public API changes, you mean that it could break semver with a breaking change? If that's the case, should we keep Breaking change as an item, or should we distinguish between these two: User changes and Developer changes? I personally like the first one, because in my mind it directly connects with an API change that client code must be aware of.
I just did give the current template a try: #15422
For the testing part I wonder if we want to encourage people to use headers for different scenarios/grouping. Do we plan to use the
How to test this PR locally?section for the test plan?
I'd say that both the review process and the test plan would benefit of having this kind of information. Besides that, explaining how to test something new would make us realise how easy/difficult is to exercise the new code. And not only that, tech writers and end users would read in that section how the software behaves, which is great in terms of gathering expectations.
About multiple test scenarios/groupings, yes, having subheaders (H3, H4...) will definitively help. We can add that to the comments to provide more info
There was a problem hiding this comment.
When you say a public API changes, you mean that it could break semver with a breaking change? If that's the case, should we keep Breaking change as an item, or should we distinguish between these two: User changes and Developer changes? I personally like the first one, because in my mind it directly connects with an API change that client code must be aware of.
I wonder what is the motivation using these fields? e.g. could labels also do? Do you plan to process these fields somehow in the future?
We don't really follow semver, because we have to follow common Elastic Stack product versioning. Which means that even minor versions can have breaking changes in the API. We track two changelogs in the Beats project. The actual user facing changelog (Changelog.asciidoc and Changelog.next.asciidoc), and a developer facing one (Changelog-developer.asciidoc and Changelog-developer.next.asciidoc). The developer one tracks changed/fixes/added/breaking.
PRs in Beats sometimes span multiple products. Changes can be developer + user facing at the same time or not at all. That is PRs are not always this well separable by product.
e.g. We can have a 'New feature' in some Beats only while having a 'breaking change' and/or 'new feature' in some API community authors might use in their own Beats.
There was a problem hiding this comment.
I wonder what is the motivation using these fields? e.g. could labels also do? Do you plan to process these fields somehow in the future?
Having labels could work too. My preference about having the fields is that is less forgettable than the labels. Anyway, Labels could be used to track issue/pr types.
We don't really follow semver, because we have to follow common Elastic Stack product versioning. Which means that even minor versions can have breaking changes in the API. We track two changelogs in the Beats project. The actual user facing changelog (Changelog.asciidoc and Changelog.next.asciidoc), and a developer facing one (Changelog-developer.asciidoc and Changelog-developer.next.asciidoc). The developer one tracks changed/fixes/added/breaking.
PRs in Beats sometimes span multiple products. Changes can be developer + user facing at the same time or not at all. That is PRs are not always this well separable by product.
e.g. We can have a 'New feature' in some Beats only while having a 'breaking change' and/or 'new feature' in some API community authors might use in their own Beats.
I guess this behavior would apply to labels too 🤔
In any case, we are free to modify/remove any section in this proposal. The goal is to create template which provides great context, fitting Beats unique SDLC. So feel free to suggest adapting the Type section in the manner it works better for the team.
There was a problem hiding this comment.
If we don't plan to use it for automating the release notes I think we should keep it more simple (meaning the way you proposed). The only type I'm missing here is 'Deprecation'.
Having labels could work too. My preference about having the fields is that is less forgettable than the labels.
+1
There was a problem hiding this comment.
How about requiring these labels to be present with a bot? I've seen this in other projects and works fairly well. Good message why the PR is not passing + instructions on how to add the right label
There was a problem hiding this comment.
We could add a comment about how to properly label the PR.
About automating the release notes/changelog, this is a topic we the @elastic/observablt-robots team are working on. We are trying to get a good automated process for that, but not there yet.
There was a problem hiding this comment.
The bot can add a label based on multiple things. I can imagine:
- if a modified path, then add label Foo (we created this bot!)
- if modified path, then ping Foo user/team
- if not following a structure, then comment the PR
- ...
|
I just did give the current template a try: #15422 For the testing part I wonder if we want to encourage people to use headers for different scenarios/grouping. Do we plan to use the |
18 tasks
exekias
reviewed
Jan 14, 2020
exekias
reviewed
Jan 14, 2020
.github/PULL_REQUEST_TEMPLATE.md
Outdated
Comment on lines
39
to
40
| - [ ] New and existing **unit** tests pass locally with my changes | ||
| - [ ] New and existing **integration** tests pass locally with my changes |
There was a problem hiding this comment.
CI should answer these questions
There was a problem hiding this comment.
Well, we's expect the sender to run the tests locally first, right? At least the unit tests
There was a problem hiding this comment.
Once the PR is created it will be run by CI anyways. I think it is good practice to run unit tests before pushing the PR, but in the end CI will tell us the truth.
I like the idea of the check list, so to not forget some common tasks (e.g. have you updated docs/configs). But I would prefer have the check list small. Things that are automatically checked anyways don't need to be in the list.
There was a problem hiding this comment.
Agreed in the shortness of the checklist is a must. We'll remove what is not necessary.
About the unit test part, I'd suggest keeping it, or at least point to the place where it explains how to run them with a nice experience.
exekias
reviewed
Jan 14, 2020
exekias
reviewed
Jan 14, 2020
|
Thank you for starting this! I left a few comments, I commented on some of the points in the checklist as I think it would make sense to keep it short if possible |
urso
reviewed
Jan 14, 2020
Yes, this is a proposal, so please feel free to cut whatever is not useful for you. At the end of the day, simple is better, so please ✂️ if you consider it |
|
I've updated the PR with your comments. Thank you for all the feedback in here :) Thanks! |
urso
reviewed
Jan 15, 2020
urso
reviewed
Jan 15, 2020
exekias
approved these changes
Jan 16, 2020
10 tasks
7 tasks
mtojek
added a commit
that referenced
this pull request
Jan 31, 2020
* [Filebeat] Fixes for NetFlow v9 devices from various vendors (#15449) - Allow for zero scope fields in options template NetFlow v9 spec allows for options templates that contain no scope fields. The netflow input was treating this case as an error and discarding the template, but that is only applicable to IPFIX. - Use additional fields to populate bytes/pkt counters Some devices out there (Cisco NSEL) use fields 231/232 as bytes counters, when those are supposed to be layer 4 payload counters. This updates the ECS fields populator to use those fields when the expected ones are not found. - Support a classId of 32 bits While the spec mandates a classId of 8 bits, some Cisco ASA devices actually use a 32 bit version of this field. This patches the field to allow up to 32-bit integers and updates the index pattern to use `long` for the `netflow.class_id` field. - Add more fields from v9 Cisco devices Fixes #14212 * update settings for `decode_csv_fields` (#15249) (#15550) Co-authored-by: DeDe Morton <[email protected]> Co-authored-by: Sophia Xu <[email protected]> * docs: updates to output config (#15272) * [Filebeat] Handle error message in handleS3Objects function (#15545) * Handle error message in handleS3Objects function * remove s3Context.Fail and use setError and done instead * Add changelog * Fix use of wrong fields in Cisco ASA dashboard (#15553) This dashboard wasn't updated after a couple of fields were renamed. Fixes: #15420 * Add test for publisher spool encode and decode. (#15534) * Add test for publisher queue encode and decode. * Run mage fmt. * Fixes from code review. * Fix convert processor conversion of string with leading zeros to integer (#15557) The conversion failed when for strings with leading zeroes and a decimal digit 8 or 9, as the underlying runtime function would try to parse that as an octal number. This is fixed by only allowing decimal and hex, which in turns makes the processor more aligned to its Elasticsearch counterpart. Fixes #15513 * New mage target: generate pkg file to test the manager (#15580) This PR adds a new mage target to Functionbeat named `buildPkgForFunction`. It generates the folder `pkg` with the functions to make testing the manager more comfortable during development. * Packetbeat TLS: Replace array fields with keyword (#15597) Use of `type: array` in some fields (which was inconsistent) causes those fields to be excluded from the template. This prevents pointing aliases to those fields, which we need in 7.6+. Setting those fields to `keyword` explicitly so that they are included in the template. Fixes #15588 * Add a pull request template providing valuable information when reviewing a PR (#15388) * Add a PR template that provides valuable information when reviewing a PR * Add CLA check * Fix typo * Address comments during review * SF: Fix typo * Add deprecation as PR type * Make it clear how to strike through in markdown * Add default configuration files to the checklist * [Metricbeat] Implement IBM MQ module (#15301) * Modify cockroachdb source * Define testdata * Do not publish ports * Update docs * mage fmt update * Describe containerized environment * Update CHANGELOG.next.asciidoc Co-Authored-By: Chris Mark <[email protected]> * Update data.json * Rename image * Update source after review * Filter ibmmq_ metrics * mage check * Fix: mage check * Don't expose port * Rename status to qmgr * Add subscriptions overview dashboard for IBM MQ module * Add calls, messages overview dashboard for IBM MQ module * Add screenshots * Fix: mage check * Fix: CHANGELOG * Add explanation * Fix: mage check Co-authored-by: Chris Mark <[email protected]> * Cleanup changelogs for master (#15617) * Cleanup changelogs for master * Remove extra header in CHANGELOG.asciidoc * [Metricbeat] Add lambda metricset in aws module (#15260) * Add lambda metricset * Adds missing imports (#15624) * [docs] Clarify privileges required for the writer role (#15604) * Mask password discovered via module autodiscover hint (#15616) * Mask password is string representation of config * Rename method * Adding unit test * Use const for module config password setting name * Using common.DebugString * Simplifying * Removing now-invalid unit test * Removing now-unnecessary const * Refactoring: moving debug-related var and func to common file * Refactoring: rename from black list to mask list * Implement fmt.Formatter for common.MapStr * Reintroduce debug statement * Make MarshalLogObject always filter MapStr object for logging purposes * Refactoring: renaming to be bit more generic * Forgot to add license header to new file * Fixing verb syntax * Update KQL to get estimated cost without dimension ServiceName (#15640) * Adding monitoring.cloud.* settings to reference files (#15648) * Adding monitoring.cloud.* settings to reference files * Missed winlogbeat somehow * Missed x-pack/winlogbeat * remove lablels (#15644) * Fix panic: don't send events if client is nil (#15568) * Fix panic: don't send events if client is nil * Use mutex * Add CHANGELOG entry * Rename changelog entry * Fix: changelog * Temporarily use specific logstash release * [Metricbeat] Add Istio mesh metricset (#15535) * [Metricbeat] Fix changelog (#15681) * Fix changelog * ci: use APM pipeline library (#15636) it uses APM pipeline library configured in the instance * AWS Lambda: downgrade Kibana dashboard (#15682) * AWS Lambda: downgrade Kibana dashboard * Downgrade other AWS dashboards * Log command error if setup dashboards fails * Another downgrade * Use github.com/godror/godror instead of goracle.v2 (#15683) From the README of goracle: > Goracle is deprecated because of naming (trademark) issues. From now on we are using github.com/godror/godror instead. * Move pdh query to shared location in order for new modules/metricsets to reuse (#15503) * Move pdh query to shared location * Update changelog * Fix make update * mage fmt * fix changelog * Remove datasource option from SQL module and add tests (#15686) Remove datasource option from SQL module. This option was intended to set the DSN of a database connection, and we were ignoring the hosts setting. In other SQL modules we are using the values in hosts as DSNs, do here the same for consistency. Host is redacted when we cannot parse it as it can contain passwords. StandardizeEvent is exposed in mbtest.Fetcher interface so we can more easily check contents of events in tests. Add integration tests of the module with MySQL and PostgreSQL. Add real data.json with data from MySQL and PostgreSQL. * [metricbeat] add service metricset to reference documentation (#15643) * add service metricset to ref docs * update xpack docs * [metricbeat] Add divide by zero check to docker/diskio (#15649) * add NaN check to docker/diskio * return 0 * Change joda style pattern to java style (#15695) since 7.0 elasticsearch is using java.time style patterns. YYYY becomes yyyy * [DOCS] Add missing config options to shared file (#15136) * [DOCS] Add missing config options to shared file * Add fixes from review * Run mage fmt update to fix build error * [Heartbeat] Support for multiple status codes #13595 (#15587) Allow for multiple status codes in config. Fixes #13595 * Add missing changelog entry for #15587 (#15721) * Update github.com/godror/godror to v0.10.4 (#15737) ## What does this PR do? This PR updates the dependency `github.com/godror/godror` to v0.10.4. ## Why is it important? Packaging of Metricbeat fails due to the issue in the `godror` version we are currently using. See more about the problem: godror/godror#8 * Collect normalized CPU percentages by default (#15729) * Collect normalized CPU percentages by default * Adding CHANGELOG entry * Updating x-pack/metricbeat * Fix: mage check * Detect Eclipse Public License Co-authored-by: Adrian Serrano <[email protected]> Co-authored-by: DeDe Morton <[email protected]> Co-authored-by: Sophia Xu <[email protected]> Co-authored-by: Brandon Morelli <[email protected]> Co-authored-by: kaiyan-sheng <[email protected]> Co-authored-by: Blake Rouse <[email protected]> Co-authored-by: Noémi Ványi <[email protected]> Co-authored-by: Manuel de la Peña <[email protected]> Co-authored-by: Chris Mark <[email protected]> Co-authored-by: Michael Madden <[email protected]> Co-authored-by: Shaunak Kashyap <[email protected]> Co-authored-by: Pablo Mercado <[email protected]> Co-authored-by: Ivan Fernandez Calvo <[email protected]> Co-authored-by: Mariana Dima <[email protected]> Co-authored-by: Jaime Soriano Pastor <[email protected]> Co-authored-by: Alex K. <[email protected]> Co-authored-by: Przemyslaw Gomulka <[email protected]> Co-authored-by: Amanda H. L. de Andrade Katz <[email protected]> Co-authored-by: Andrew Cholakian <[email protected]>
mtojek
added a commit
that referenced
this pull request
Feb 12, 2020
* Add MQTT input to Filebeat (#15287) * Inital commit for MQTT input * Improved naming and error handling * Improved naming and connection procedure * Merge "master" branch into "feature-mqtt-input" (#15745) * [Filebeat] Fixes for NetFlow v9 devices from various vendors (#15449) - Allow for zero scope fields in options template NetFlow v9 spec allows for options templates that contain no scope fields. The netflow input was treating this case as an error and discarding the template, but that is only applicable to IPFIX. - Use additional fields to populate bytes/pkt counters Some devices out there (Cisco NSEL) use fields 231/232 as bytes counters, when those are supposed to be layer 4 payload counters. This updates the ECS fields populator to use those fields when the expected ones are not found. - Support a classId of 32 bits While the spec mandates a classId of 8 bits, some Cisco ASA devices actually use a 32 bit version of this field. This patches the field to allow up to 32-bit integers and updates the index pattern to use `long` for the `netflow.class_id` field. - Add more fields from v9 Cisco devices Fixes #14212 * update settings for `decode_csv_fields` (#15249) (#15550) Co-authored-by: DeDe Morton <[email protected]> Co-authored-by: Sophia Xu <[email protected]> * docs: updates to output config (#15272) * [Filebeat] Handle error message in handleS3Objects function (#15545) * Handle error message in handleS3Objects function * remove s3Context.Fail and use setError and done instead * Add changelog * Fix use of wrong fields in Cisco ASA dashboard (#15553) This dashboard wasn't updated after a couple of fields were renamed. Fixes: #15420 * Add test for publisher spool encode and decode. (#15534) * Add test for publisher queue encode and decode. * Run mage fmt. * Fixes from code review. * Fix convert processor conversion of string with leading zeros to integer (#15557) The conversion failed when for strings with leading zeroes and a decimal digit 8 or 9, as the underlying runtime function would try to parse that as an octal number. This is fixed by only allowing decimal and hex, which in turns makes the processor more aligned to its Elasticsearch counterpart. Fixes #15513 * New mage target: generate pkg file to test the manager (#15580) This PR adds a new mage target to Functionbeat named `buildPkgForFunction`. It generates the folder `pkg` with the functions to make testing the manager more comfortable during development. * Packetbeat TLS: Replace array fields with keyword (#15597) Use of `type: array` in some fields (which was inconsistent) causes those fields to be excluded from the template. This prevents pointing aliases to those fields, which we need in 7.6+. Setting those fields to `keyword` explicitly so that they are included in the template. Fixes #15588 * Add a pull request template providing valuable information when reviewing a PR (#15388) * Add a PR template that provides valuable information when reviewing a PR * Add CLA check * Fix typo * Address comments during review * SF: Fix typo * Add deprecation as PR type * Make it clear how to strike through in markdown * Add default configuration files to the checklist * [Metricbeat] Implement IBM MQ module (#15301) * Modify cockroachdb source * Define testdata * Do not publish ports * Update docs * mage fmt update * Describe containerized environment * Update CHANGELOG.next.asciidoc Co-Authored-By: Chris Mark <[email protected]> * Update data.json * Rename image * Update source after review * Filter ibmmq_ metrics * mage check * Fix: mage check * Don't expose port * Rename status to qmgr * Add subscriptions overview dashboard for IBM MQ module * Add calls, messages overview dashboard for IBM MQ module * Add screenshots * Fix: mage check * Fix: CHANGELOG * Add explanation * Fix: mage check Co-authored-by: Chris Mark <[email protected]> * Cleanup changelogs for master (#15617) * Cleanup changelogs for master * Remove extra header in CHANGELOG.asciidoc * [Metricbeat] Add lambda metricset in aws module (#15260) * Add lambda metricset * Adds missing imports (#15624) * [docs] Clarify privileges required for the writer role (#15604) * Mask password discovered via module autodiscover hint (#15616) * Mask password is string representation of config * Rename method * Adding unit test * Use const for module config password setting name * Using common.DebugString * Simplifying * Removing now-invalid unit test * Removing now-unnecessary const * Refactoring: moving debug-related var and func to common file * Refactoring: rename from black list to mask list * Implement fmt.Formatter for common.MapStr * Reintroduce debug statement * Make MarshalLogObject always filter MapStr object for logging purposes * Refactoring: renaming to be bit more generic * Forgot to add license header to new file * Fixing verb syntax * Update KQL to get estimated cost without dimension ServiceName (#15640) * Adding monitoring.cloud.* settings to reference files (#15648) * Adding monitoring.cloud.* settings to reference files * Missed winlogbeat somehow * Missed x-pack/winlogbeat * remove lablels (#15644) * Fix panic: don't send events if client is nil (#15568) * Fix panic: don't send events if client is nil * Use mutex * Add CHANGELOG entry * Rename changelog entry * Fix: changelog * Temporarily use specific logstash release * [Metricbeat] Add Istio mesh metricset (#15535) * [Metricbeat] Fix changelog (#15681) * Fix changelog * ci: use APM pipeline library (#15636) it uses APM pipeline library configured in the instance * AWS Lambda: downgrade Kibana dashboard (#15682) * AWS Lambda: downgrade Kibana dashboard * Downgrade other AWS dashboards * Log command error if setup dashboards fails * Another downgrade * Use github.com/godror/godror instead of goracle.v2 (#15683) From the README of goracle: > Goracle is deprecated because of naming (trademark) issues. From now on we are using github.com/godror/godror instead. * Move pdh query to shared location in order for new modules/metricsets to reuse (#15503) * Move pdh query to shared location * Update changelog * Fix make update * mage fmt * fix changelog * Remove datasource option from SQL module and add tests (#15686) Remove datasource option from SQL module. This option was intended to set the DSN of a database connection, and we were ignoring the hosts setting. In other SQL modules we are using the values in hosts as DSNs, do here the same for consistency. Host is redacted when we cannot parse it as it can contain passwords. StandardizeEvent is exposed in mbtest.Fetcher interface so we can more easily check contents of events in tests. Add integration tests of the module with MySQL and PostgreSQL. Add real data.json with data from MySQL and PostgreSQL. * [metricbeat] add service metricset to reference documentation (#15643) * add service metricset to ref docs * update xpack docs * [metricbeat] Add divide by zero check to docker/diskio (#15649) * add NaN check to docker/diskio * return 0 * Change joda style pattern to java style (#15695) since 7.0 elasticsearch is using java.time style patterns. YYYY becomes yyyy * [DOCS] Add missing config options to shared file (#15136) * [DOCS] Add missing config options to shared file * Add fixes from review * Run mage fmt update to fix build error * [Heartbeat] Support for multiple status codes #13595 (#15587) Allow for multiple status codes in config. Fixes #13595 * Add missing changelog entry for #15587 (#15721) * Update github.com/godror/godror to v0.10.4 (#15737) ## What does this PR do? This PR updates the dependency `github.com/godror/godror` to v0.10.4. ## Why is it important? Packaging of Metricbeat fails due to the issue in the `godror` version we are currently using. See more about the problem: godror/godror#8 * Collect normalized CPU percentages by default (#15729) * Collect normalized CPU percentages by default * Adding CHANGELOG entry * Updating x-pack/metricbeat * Fix: mage check * Detect Eclipse Public License Co-authored-by: Adrian Serrano <[email protected]> Co-authored-by: DeDe Morton <[email protected]> Co-authored-by: Sophia Xu <[email protected]> Co-authored-by: Brandon Morelli <[email protected]> Co-authored-by: kaiyan-sheng <[email protected]> Co-authored-by: Blake Rouse <[email protected]> Co-authored-by: Noémi Ványi <[email protected]> Co-authored-by: Manuel de la Peña <[email protected]> Co-authored-by: Chris Mark <[email protected]> Co-authored-by: Michael Madden <[email protected]> Co-authored-by: Shaunak Kashyap <[email protected]> Co-authored-by: Pablo Mercado <[email protected]> Co-authored-by: Ivan Fernandez Calvo <[email protected]> Co-authored-by: Mariana Dima <[email protected]> Co-authored-by: Jaime Soriano Pastor <[email protected]> Co-authored-by: Alex K. <[email protected]> Co-authored-by: Przemyslaw Gomulka <[email protected]> Co-authored-by: Amanda H. L. de Andrade Katz <[email protected]> Co-authored-by: Andrew Cholakian <[email protected]> * [Filebeat] Refactor mqtt input (#16014) * Refactor mqtt input * Fix: comment * Add unit tests * Test: input run * Fix Test: run and stop * Test: backoff * Adjust code after review * MQTT: update docs (#16152) * MQTT: add integration test (#16143) * Create mosquitto image * MQTT input: add integration test * Fix * Verify connectivity * Fix * Fix: mage check * Fix * Fix * Fix: remove global var * Update changelog * Fix: regenerate notice file * Remove unused dependency * Fix: zero qos * Wait asynchronously for client being disconnected Co-authored-by: Felix <[email protected]> Co-authored-by: Adrian Serrano <[email protected]> Co-authored-by: DeDe Morton <[email protected]> Co-authored-by: Sophia Xu <[email protected]> Co-authored-by: Brandon Morelli <[email protected]> Co-authored-by: kaiyan-sheng <[email protected]> Co-authored-by: Blake Rouse <[email protected]> Co-authored-by: Noémi Ványi <[email protected]> Co-authored-by: Manuel de la Peña <[email protected]> Co-authored-by: Chris Mark <[email protected]> Co-authored-by: Michael Madden <[email protected]> Co-authored-by: Shaunak Kashyap <[email protected]> Co-authored-by: Pablo Mercado <[email protected]> Co-authored-by: Ivan Fernandez Calvo <[email protected]> Co-authored-by: Mariana Dima <[email protected]> Co-authored-by: Jaime Soriano Pastor <[email protected]> Co-authored-by: Alex K. <[email protected]> Co-authored-by: Przemyslaw Gomulka <[email protected]> Co-authored-by: Amanda H. L. de Andrade Katz <[email protected]> Co-authored-by: Andrew Cholakian <[email protected]>
mtojek
added a commit
to mtojek/beats
that referenced
this pull request
Feb 12, 2020
* Add MQTT input to Filebeat (elastic#15287) * Inital commit for MQTT input * Improved naming and error handling * Improved naming and connection procedure * Merge "master" branch into "feature-mqtt-input" (elastic#15745) * [Filebeat] Fixes for NetFlow v9 devices from various vendors (elastic#15449) - Allow for zero scope fields in options template NetFlow v9 spec allows for options templates that contain no scope fields. The netflow input was treating this case as an error and discarding the template, but that is only applicable to IPFIX. - Use additional fields to populate bytes/pkt counters Some devices out there (Cisco NSEL) use fields 231/232 as bytes counters, when those are supposed to be layer 4 payload counters. This updates the ECS fields populator to use those fields when the expected ones are not found. - Support a classId of 32 bits While the spec mandates a classId of 8 bits, some Cisco ASA devices actually use a 32 bit version of this field. This patches the field to allow up to 32-bit integers and updates the index pattern to use `long` for the `netflow.class_id` field. - Add more fields from v9 Cisco devices Fixes elastic#14212 * update settings for `decode_csv_fields` (elastic#15249) (elastic#15550) Co-authored-by: DeDe Morton <[email protected]> Co-authored-by: Sophia Xu <[email protected]> * docs: updates to output config (elastic#15272) * [Filebeat] Handle error message in handleS3Objects function (elastic#15545) * Handle error message in handleS3Objects function * remove s3Context.Fail and use setError and done instead * Add changelog * Fix use of wrong fields in Cisco ASA dashboard (elastic#15553) This dashboard wasn't updated after a couple of fields were renamed. Fixes: elastic#15420 * Add test for publisher spool encode and decode. (elastic#15534) * Add test for publisher queue encode and decode. * Run mage fmt. * Fixes from code review. * Fix convert processor conversion of string with leading zeros to integer (elastic#15557) The conversion failed when for strings with leading zeroes and a decimal digit 8 or 9, as the underlying runtime function would try to parse that as an octal number. This is fixed by only allowing decimal and hex, which in turns makes the processor more aligned to its Elasticsearch counterpart. Fixes elastic#15513 * New mage target: generate pkg file to test the manager (elastic#15580) This PR adds a new mage target to Functionbeat named `buildPkgForFunction`. It generates the folder `pkg` with the functions to make testing the manager more comfortable during development. * Packetbeat TLS: Replace array fields with keyword (elastic#15597) Use of `type: array` in some fields (which was inconsistent) causes those fields to be excluded from the template. This prevents pointing aliases to those fields, which we need in 7.6+. Setting those fields to `keyword` explicitly so that they are included in the template. Fixes elastic#15588 * Add a pull request template providing valuable information when reviewing a PR (elastic#15388) * Add a PR template that provides valuable information when reviewing a PR * Add CLA check * Fix typo * Address comments during review * SF: Fix typo * Add deprecation as PR type * Make it clear how to strike through in markdown * Add default configuration files to the checklist * [Metricbeat] Implement IBM MQ module (elastic#15301) * Modify cockroachdb source * Define testdata * Do not publish ports * Update docs * mage fmt update * Describe containerized environment * Update CHANGELOG.next.asciidoc Co-Authored-By: Chris Mark <[email protected]> * Update data.json * Rename image * Update source after review * Filter ibmmq_ metrics * mage check * Fix: mage check * Don't expose port * Rename status to qmgr * Add subscriptions overview dashboard for IBM MQ module * Add calls, messages overview dashboard for IBM MQ module * Add screenshots * Fix: mage check * Fix: CHANGELOG * Add explanation * Fix: mage check Co-authored-by: Chris Mark <[email protected]> * Cleanup changelogs for master (elastic#15617) * Cleanup changelogs for master * Remove extra header in CHANGELOG.asciidoc * [Metricbeat] Add lambda metricset in aws module (elastic#15260) * Add lambda metricset * Adds missing imports (elastic#15624) * [docs] Clarify privileges required for the writer role (elastic#15604) * Mask password discovered via module autodiscover hint (elastic#15616) * Mask password is string representation of config * Rename method * Adding unit test * Use const for module config password setting name * Using common.DebugString * Simplifying * Removing now-invalid unit test * Removing now-unnecessary const * Refactoring: moving debug-related var and func to common file * Refactoring: rename from black list to mask list * Implement fmt.Formatter for common.MapStr * Reintroduce debug statement * Make MarshalLogObject always filter MapStr object for logging purposes * Refactoring: renaming to be bit more generic * Forgot to add license header to new file * Fixing verb syntax * Update KQL to get estimated cost without dimension ServiceName (elastic#15640) * Adding monitoring.cloud.* settings to reference files (elastic#15648) * Adding monitoring.cloud.* settings to reference files * Missed winlogbeat somehow * Missed x-pack/winlogbeat * remove lablels (elastic#15644) * Fix panic: don't send events if client is nil (elastic#15568) * Fix panic: don't send events if client is nil * Use mutex * Add CHANGELOG entry * Rename changelog entry * Fix: changelog * Temporarily use specific logstash release * [Metricbeat] Add Istio mesh metricset (elastic#15535) * [Metricbeat] Fix changelog (elastic#15681) * Fix changelog * ci: use APM pipeline library (elastic#15636) it uses APM pipeline library configured in the instance * AWS Lambda: downgrade Kibana dashboard (elastic#15682) * AWS Lambda: downgrade Kibana dashboard * Downgrade other AWS dashboards * Log command error if setup dashboards fails * Another downgrade * Use github.com/godror/godror instead of goracle.v2 (elastic#15683) From the README of goracle: > Goracle is deprecated because of naming (trademark) issues. From now on we are using github.com/godror/godror instead. * Move pdh query to shared location in order for new modules/metricsets to reuse (elastic#15503) * Move pdh query to shared location * Update changelog * Fix make update * mage fmt * fix changelog * Remove datasource option from SQL module and add tests (elastic#15686) Remove datasource option from SQL module. This option was intended to set the DSN of a database connection, and we were ignoring the hosts setting. In other SQL modules we are using the values in hosts as DSNs, do here the same for consistency. Host is redacted when we cannot parse it as it can contain passwords. StandardizeEvent is exposed in mbtest.Fetcher interface so we can more easily check contents of events in tests. Add integration tests of the module with MySQL and PostgreSQL. Add real data.json with data from MySQL and PostgreSQL. * [metricbeat] add service metricset to reference documentation (elastic#15643) * add service metricset to ref docs * update xpack docs * [metricbeat] Add divide by zero check to docker/diskio (elastic#15649) * add NaN check to docker/diskio * return 0 * Change joda style pattern to java style (elastic#15695) since 7.0 elasticsearch is using java.time style patterns. YYYY becomes yyyy * [DOCS] Add missing config options to shared file (elastic#15136) * [DOCS] Add missing config options to shared file * Add fixes from review * Run mage fmt update to fix build error * [Heartbeat] Support for multiple status codes elastic#13595 (elastic#15587) Allow for multiple status codes in config. Fixes elastic#13595 * Add missing changelog entry for elastic#15587 (elastic#15721) * Update github.com/godror/godror to v0.10.4 (elastic#15737) ## What does this PR do? This PR updates the dependency `github.com/godror/godror` to v0.10.4. ## Why is it important? Packaging of Metricbeat fails due to the issue in the `godror` version we are currently using. See more about the problem: godror/godror#8 * Collect normalized CPU percentages by default (elastic#15729) * Collect normalized CPU percentages by default * Adding CHANGELOG entry * Updating x-pack/metricbeat * Fix: mage check * Detect Eclipse Public License Co-authored-by: Adrian Serrano <[email protected]> Co-authored-by: DeDe Morton <[email protected]> Co-authored-by: Sophia Xu <[email protected]> Co-authored-by: Brandon Morelli <[email protected]> Co-authored-by: kaiyan-sheng <[email protected]> Co-authored-by: Blake Rouse <[email protected]> Co-authored-by: Noémi Ványi <[email protected]> Co-authored-by: Manuel de la Peña <[email protected]> Co-authored-by: Chris Mark <[email protected]> Co-authored-by: Michael Madden <[email protected]> Co-authored-by: Shaunak Kashyap <[email protected]> Co-authored-by: Pablo Mercado <[email protected]> Co-authored-by: Ivan Fernandez Calvo <[email protected]> Co-authored-by: Mariana Dima <[email protected]> Co-authored-by: Jaime Soriano Pastor <[email protected]> Co-authored-by: Alex K. <[email protected]> Co-authored-by: Przemyslaw Gomulka <[email protected]> Co-authored-by: Amanda H. L. de Andrade Katz <[email protected]> Co-authored-by: Andrew Cholakian <[email protected]> * [Filebeat] Refactor mqtt input (elastic#16014) * Refactor mqtt input * Fix: comment * Add unit tests * Test: input run * Fix Test: run and stop * Test: backoff * Adjust code after review * MQTT: update docs (elastic#16152) * MQTT: add integration test (elastic#16143) * Create mosquitto image * MQTT input: add integration test * Fix * Verify connectivity * Fix * Fix: mage check * Fix * Fix * Fix: remove global var * Update changelog * Fix: regenerate notice file * Remove unused dependency * Fix: zero qos * Wait asynchronously for client being disconnected Co-authored-by: Felix <[email protected]> Co-authored-by: Adrian Serrano <[email protected]> Co-authored-by: DeDe Morton <[email protected]> Co-authored-by: Sophia Xu <[email protected]> Co-authored-by: Brandon Morelli <[email protected]> Co-authored-by: kaiyan-sheng <[email protected]> Co-authored-by: Blake Rouse <[email protected]> Co-authored-by: Noémi Ványi <[email protected]> Co-authored-by: Manuel de la Peña <[email protected]> Co-authored-by: Chris Mark <[email protected]> Co-authored-by: Michael Madden <[email protected]> Co-authored-by: Shaunak Kashyap <[email protected]> Co-authored-by: Pablo Mercado <[email protected]> Co-authored-by: Ivan Fernandez Calvo <[email protected]> Co-authored-by: Mariana Dima <[email protected]> Co-authored-by: Jaime Soriano Pastor <[email protected]> Co-authored-by: Alex K. <[email protected]> Co-authored-by: Przemyslaw Gomulka <[email protected]> Co-authored-by: Amanda H. L. de Andrade Katz <[email protected]> Co-authored-by: Andrew Cholakian <[email protected]> (cherry picked from commit 56c93e5)
mtojek
added a commit
that referenced
this pull request
Feb 12, 2020
…6284) * Filebeat: Merge "mqtt" input to master (#16204) * Add MQTT input to Filebeat (#15287) * Inital commit for MQTT input * Improved naming and error handling * Improved naming and connection procedure * Merge "master" branch into "feature-mqtt-input" (#15745) * [Filebeat] Fixes for NetFlow v9 devices from various vendors (#15449) - Allow for zero scope fields in options template NetFlow v9 spec allows for options templates that contain no scope fields. The netflow input was treating this case as an error and discarding the template, but that is only applicable to IPFIX. - Use additional fields to populate bytes/pkt counters Some devices out there (Cisco NSEL) use fields 231/232 as bytes counters, when those are supposed to be layer 4 payload counters. This updates the ECS fields populator to use those fields when the expected ones are not found. - Support a classId of 32 bits While the spec mandates a classId of 8 bits, some Cisco ASA devices actually use a 32 bit version of this field. This patches the field to allow up to 32-bit integers and updates the index pattern to use `long` for the `netflow.class_id` field. - Add more fields from v9 Cisco devices Fixes #14212 * update settings for `decode_csv_fields` (#15249) (#15550) Co-authored-by: DeDe Morton <[email protected]> Co-authored-by: Sophia Xu <[email protected]> * docs: updates to output config (#15272) * [Filebeat] Handle error message in handleS3Objects function (#15545) * Handle error message in handleS3Objects function * remove s3Context.Fail and use setError and done instead * Add changelog * Fix use of wrong fields in Cisco ASA dashboard (#15553) This dashboard wasn't updated after a couple of fields were renamed. Fixes: #15420 * Add test for publisher spool encode and decode. (#15534) * Add test for publisher queue encode and decode. * Run mage fmt. * Fixes from code review. * Fix convert processor conversion of string with leading zeros to integer (#15557) The conversion failed when for strings with leading zeroes and a decimal digit 8 or 9, as the underlying runtime function would try to parse that as an octal number. This is fixed by only allowing decimal and hex, which in turns makes the processor more aligned to its Elasticsearch counterpart. Fixes #15513 * New mage target: generate pkg file to test the manager (#15580) This PR adds a new mage target to Functionbeat named `buildPkgForFunction`. It generates the folder `pkg` with the functions to make testing the manager more comfortable during development. * Packetbeat TLS: Replace array fields with keyword (#15597) Use of `type: array` in some fields (which was inconsistent) causes those fields to be excluded from the template. This prevents pointing aliases to those fields, which we need in 7.6+. Setting those fields to `keyword` explicitly so that they are included in the template. Fixes #15588 * Add a pull request template providing valuable information when reviewing a PR (#15388) * Add a PR template that provides valuable information when reviewing a PR * Add CLA check * Fix typo * Address comments during review * SF: Fix typo * Add deprecation as PR type * Make it clear how to strike through in markdown * Add default configuration files to the checklist * [Metricbeat] Implement IBM MQ module (#15301) * Modify cockroachdb source * Define testdata * Do not publish ports * Update docs * mage fmt update * Describe containerized environment * Update CHANGELOG.next.asciidoc Co-Authored-By: Chris Mark <[email protected]> * Update data.json * Rename image * Update source after review * Filter ibmmq_ metrics * mage check * Fix: mage check * Don't expose port * Rename status to qmgr * Add subscriptions overview dashboard for IBM MQ module * Add calls, messages overview dashboard for IBM MQ module * Add screenshots * Fix: mage check * Fix: CHANGELOG * Add explanation * Fix: mage check Co-authored-by: Chris Mark <[email protected]> * Cleanup changelogs for master (#15617) * Cleanup changelogs for master * Remove extra header in CHANGELOG.asciidoc * [Metricbeat] Add lambda metricset in aws module (#15260) * Add lambda metricset * Adds missing imports (#15624) * [docs] Clarify privileges required for the writer role (#15604) * Mask password discovered via module autodiscover hint (#15616) * Mask password is string representation of config * Rename method * Adding unit test * Use const for module config password setting name * Using common.DebugString * Simplifying * Removing now-invalid unit test * Removing now-unnecessary const * Refactoring: moving debug-related var and func to common file * Refactoring: rename from black list to mask list * Implement fmt.Formatter for common.MapStr * Reintroduce debug statement * Make MarshalLogObject always filter MapStr object for logging purposes * Refactoring: renaming to be bit more generic * Forgot to add license header to new file * Fixing verb syntax * Update KQL to get estimated cost without dimension ServiceName (#15640) * Adding monitoring.cloud.* settings to reference files (#15648) * Adding monitoring.cloud.* settings to reference files * Missed winlogbeat somehow * Missed x-pack/winlogbeat * remove lablels (#15644) * Fix panic: don't send events if client is nil (#15568) * Fix panic: don't send events if client is nil * Use mutex * Add CHANGELOG entry * Rename changelog entry * Fix: changelog * Temporarily use specific logstash release * [Metricbeat] Add Istio mesh metricset (#15535) * [Metricbeat] Fix changelog (#15681) * Fix changelog * ci: use APM pipeline library (#15636) it uses APM pipeline library configured in the instance * AWS Lambda: downgrade Kibana dashboard (#15682) * AWS Lambda: downgrade Kibana dashboard * Downgrade other AWS dashboards * Log command error if setup dashboards fails * Another downgrade * Use github.com/godror/godror instead of goracle.v2 (#15683) From the README of goracle: > Goracle is deprecated because of naming (trademark) issues. From now on we are using github.com/godror/godror instead. * Move pdh query to shared location in order for new modules/metricsets to reuse (#15503) * Move pdh query to shared location * Update changelog * Fix make update * mage fmt * fix changelog * Remove datasource option from SQL module and add tests (#15686) Remove datasource option from SQL module. This option was intended to set the DSN of a database connection, and we were ignoring the hosts setting. In other SQL modules we are using the values in hosts as DSNs, do here the same for consistency. Host is redacted when we cannot parse it as it can contain passwords. StandardizeEvent is exposed in mbtest.Fetcher interface so we can more easily check contents of events in tests. Add integration tests of the module with MySQL and PostgreSQL. Add real data.json with data from MySQL and PostgreSQL. * [metricbeat] add service metricset to reference documentation (#15643) * add service metricset to ref docs * update xpack docs * [metricbeat] Add divide by zero check to docker/diskio (#15649) * add NaN check to docker/diskio * return 0 * Change joda style pattern to java style (#15695) since 7.0 elasticsearch is using java.time style patterns. YYYY becomes yyyy * [DOCS] Add missing config options to shared file (#15136) * [DOCS] Add missing config options to shared file * Add fixes from review * Run mage fmt update to fix build error * [Heartbeat] Support for multiple status codes #13595 (#15587) Allow for multiple status codes in config. Fixes #13595 * Add missing changelog entry for #15587 (#15721) * Update github.com/godror/godror to v0.10.4 (#15737) ## What does this PR do? This PR updates the dependency `github.com/godror/godror` to v0.10.4. ## Why is it important? Packaging of Metricbeat fails due to the issue in the `godror` version we are currently using. See more about the problem: godror/godror#8 * Collect normalized CPU percentages by default (#15729) * Collect normalized CPU percentages by default * Adding CHANGELOG entry * Updating x-pack/metricbeat * Fix: mage check * Detect Eclipse Public License Co-authored-by: Adrian Serrano <[email protected]> Co-authored-by: DeDe Morton <[email protected]> Co-authored-by: Sophia Xu <[email protected]> Co-authored-by: Brandon Morelli <[email protected]> Co-authored-by: kaiyan-sheng <[email protected]> Co-authored-by: Blake Rouse <[email protected]> Co-authored-by: Noémi Ványi <[email protected]> Co-authored-by: Manuel de la Peña <[email protected]> Co-authored-by: Chris Mark <[email protected]> Co-authored-by: Michael Madden <[email protected]> Co-authored-by: Shaunak Kashyap <[email protected]> Co-authored-by: Pablo Mercado <[email protected]> Co-authored-by: Ivan Fernandez Calvo <[email protected]> Co-authored-by: Mariana Dima <[email protected]> Co-authored-by: Jaime Soriano Pastor <[email protected]> Co-authored-by: Alex K. <[email protected]> Co-authored-by: Przemyslaw Gomulka <[email protected]> Co-authored-by: Amanda H. L. de Andrade Katz <[email protected]> Co-authored-by: Andrew Cholakian <[email protected]> * [Filebeat] Refactor mqtt input (#16014) * Refactor mqtt input * Fix: comment * Add unit tests * Test: input run * Fix Test: run and stop * Test: backoff * Adjust code after review * MQTT: update docs (#16152) * MQTT: add integration test (#16143) * Create mosquitto image * MQTT input: add integration test * Fix * Verify connectivity * Fix * Fix: mage check * Fix * Fix * Fix: remove global var * Update changelog * Fix: regenerate notice file * Remove unused dependency * Fix: zero qos * Wait asynchronously for client being disconnected Co-authored-by: Felix <[email protected]> Co-authored-by: Adrian Serrano <[email protected]> Co-authored-by: DeDe Morton <[email protected]> Co-authored-by: Sophia Xu <[email protected]> Co-authored-by: Brandon Morelli <[email protected]> Co-authored-by: kaiyan-sheng <[email protected]> Co-authored-by: Blake Rouse <[email protected]> Co-authored-by: Noémi Ványi <[email protected]> Co-authored-by: Manuel de la Peña <[email protected]> Co-authored-by: Chris Mark <[email protected]> Co-authored-by: Michael Madden <[email protected]> Co-authored-by: Shaunak Kashyap <[email protected]> Co-authored-by: Pablo Mercado <[email protected]> Co-authored-by: Ivan Fernandez Calvo <[email protected]> Co-authored-by: Mariana Dima <[email protected]> Co-authored-by: Jaime Soriano Pastor <[email protected]> Co-authored-by: Alex K. <[email protected]> Co-authored-by: Przemyslaw Gomulka <[email protected]> Co-authored-by: Amanda H. L. de Andrade Katz <[email protected]> Co-authored-by: Andrew Cholakian <[email protected]> (cherry picked from commit 56c93e5) * Update CHANGELOG
kvch
added a commit
to kvch/beats
that referenced
this pull request
Feb 20, 2020
* Add MQTT input to Filebeat (elastic#15287) * Inital commit for MQTT input * Improved naming and error handling * Improved naming and connection procedure * Merge "master" branch into "feature-mqtt-input" (elastic#15745) * [Filebeat] Fixes for NetFlow v9 devices from various vendors (elastic#15449) - Allow for zero scope fields in options template NetFlow v9 spec allows for options templates that contain no scope fields. The netflow input was treating this case as an error and discarding the template, but that is only applicable to IPFIX. - Use additional fields to populate bytes/pkt counters Some devices out there (Cisco NSEL) use fields 231/232 as bytes counters, when those are supposed to be layer 4 payload counters. This updates the ECS fields populator to use those fields when the expected ones are not found. - Support a classId of 32 bits While the spec mandates a classId of 8 bits, some Cisco ASA devices actually use a 32 bit version of this field. This patches the field to allow up to 32-bit integers and updates the index pattern to use `long` for the `netflow.class_id` field. - Add more fields from v9 Cisco devices Fixes elastic#14212 * update settings for `decode_csv_fields` (elastic#15249) (elastic#15550) Co-authored-by: DeDe Morton <[email protected]> Co-authored-by: Sophia Xu <[email protected]> * docs: updates to output config (elastic#15272) * [Filebeat] Handle error message in handleS3Objects function (elastic#15545) * Handle error message in handleS3Objects function * remove s3Context.Fail and use setError and done instead * Add changelog * Fix use of wrong fields in Cisco ASA dashboard (elastic#15553) This dashboard wasn't updated after a couple of fields were renamed. Fixes: elastic#15420 * Add test for publisher spool encode and decode. (elastic#15534) * Add test for publisher queue encode and decode. * Run mage fmt. * Fixes from code review. * Fix convert processor conversion of string with leading zeros to integer (elastic#15557) The conversion failed when for strings with leading zeroes and a decimal digit 8 or 9, as the underlying runtime function would try to parse that as an octal number. This is fixed by only allowing decimal and hex, which in turns makes the processor more aligned to its Elasticsearch counterpart. Fixes elastic#15513 * New mage target: generate pkg file to test the manager (elastic#15580) This PR adds a new mage target to Functionbeat named `buildPkgForFunction`. It generates the folder `pkg` with the functions to make testing the manager more comfortable during development. * Packetbeat TLS: Replace array fields with keyword (elastic#15597) Use of `type: array` in some fields (which was inconsistent) causes those fields to be excluded from the template. This prevents pointing aliases to those fields, which we need in 7.6+. Setting those fields to `keyword` explicitly so that they are included in the template. Fixes elastic#15588 * Add a pull request template providing valuable information when reviewing a PR (elastic#15388) * Add a PR template that provides valuable information when reviewing a PR * Add CLA check * Fix typo * Address comments during review * SF: Fix typo * Add deprecation as PR type * Make it clear how to strike through in markdown * Add default configuration files to the checklist * [Metricbeat] Implement IBM MQ module (elastic#15301) * Modify cockroachdb source * Define testdata * Do not publish ports * Update docs * mage fmt update * Describe containerized environment * Update CHANGELOG.next.asciidoc Co-Authored-By: Chris Mark <[email protected]> * Update data.json * Rename image * Update source after review * Filter ibmmq_ metrics * mage check * Fix: mage check * Don't expose port * Rename status to qmgr * Add subscriptions overview dashboard for IBM MQ module * Add calls, messages overview dashboard for IBM MQ module * Add screenshots * Fix: mage check * Fix: CHANGELOG * Add explanation * Fix: mage check Co-authored-by: Chris Mark <[email protected]> * Cleanup changelogs for master (elastic#15617) * Cleanup changelogs for master * Remove extra header in CHANGELOG.asciidoc * [Metricbeat] Add lambda metricset in aws module (elastic#15260) * Add lambda metricset * Adds missing imports (elastic#15624) * [docs] Clarify privileges required for the writer role (elastic#15604) * Mask password discovered via module autodiscover hint (elastic#15616) * Mask password is string representation of config * Rename method * Adding unit test * Use const for module config password setting name * Using common.DebugString * Simplifying * Removing now-invalid unit test * Removing now-unnecessary const * Refactoring: moving debug-related var and func to common file * Refactoring: rename from black list to mask list * Implement fmt.Formatter for common.MapStr * Reintroduce debug statement * Make MarshalLogObject always filter MapStr object for logging purposes * Refactoring: renaming to be bit more generic * Forgot to add license header to new file * Fixing verb syntax * Update KQL to get estimated cost without dimension ServiceName (elastic#15640) * Adding monitoring.cloud.* settings to reference files (elastic#15648) * Adding monitoring.cloud.* settings to reference files * Missed winlogbeat somehow * Missed x-pack/winlogbeat * remove lablels (elastic#15644) * Fix panic: don't send events if client is nil (elastic#15568) * Fix panic: don't send events if client is nil * Use mutex * Add CHANGELOG entry * Rename changelog entry * Fix: changelog * Temporarily use specific logstash release * [Metricbeat] Add Istio mesh metricset (elastic#15535) * [Metricbeat] Fix changelog (elastic#15681) * Fix changelog * ci: use APM pipeline library (elastic#15636) it uses APM pipeline library configured in the instance * AWS Lambda: downgrade Kibana dashboard (elastic#15682) * AWS Lambda: downgrade Kibana dashboard * Downgrade other AWS dashboards * Log command error if setup dashboards fails * Another downgrade * Use github.com/godror/godror instead of goracle.v2 (elastic#15683) From the README of goracle: > Goracle is deprecated because of naming (trademark) issues. From now on we are using github.com/godror/godror instead. * Move pdh query to shared location in order for new modules/metricsets to reuse (elastic#15503) * Move pdh query to shared location * Update changelog * Fix make update * mage fmt * fix changelog * Remove datasource option from SQL module and add tests (elastic#15686) Remove datasource option from SQL module. This option was intended to set the DSN of a database connection, and we were ignoring the hosts setting. In other SQL modules we are using the values in hosts as DSNs, do here the same for consistency. Host is redacted when we cannot parse it as it can contain passwords. StandardizeEvent is exposed in mbtest.Fetcher interface so we can more easily check contents of events in tests. Add integration tests of the module with MySQL and PostgreSQL. Add real data.json with data from MySQL and PostgreSQL. * [metricbeat] add service metricset to reference documentation (elastic#15643) * add service metricset to ref docs * update xpack docs * [metricbeat] Add divide by zero check to docker/diskio (elastic#15649) * add NaN check to docker/diskio * return 0 * Change joda style pattern to java style (elastic#15695) since 7.0 elasticsearch is using java.time style patterns. YYYY becomes yyyy * [DOCS] Add missing config options to shared file (elastic#15136) * [DOCS] Add missing config options to shared file * Add fixes from review * Run mage fmt update to fix build error * [Heartbeat] Support for multiple status codes elastic#13595 (elastic#15587) Allow for multiple status codes in config. Fixes elastic#13595 * Add missing changelog entry for elastic#15587 (elastic#15721) * Update github.com/godror/godror to v0.10.4 (elastic#15737) ## What does this PR do? This PR updates the dependency `github.com/godror/godror` to v0.10.4. ## Why is it important? Packaging of Metricbeat fails due to the issue in the `godror` version we are currently using. See more about the problem: godror/godror#8 * Collect normalized CPU percentages by default (elastic#15729) * Collect normalized CPU percentages by default * Adding CHANGELOG entry * Updating x-pack/metricbeat * Fix: mage check * Detect Eclipse Public License Co-authored-by: Adrian Serrano <[email protected]> Co-authored-by: DeDe Morton <[email protected]> Co-authored-by: Sophia Xu <[email protected]> Co-authored-by: Brandon Morelli <[email protected]> Co-authored-by: kaiyan-sheng <[email protected]> Co-authored-by: Blake Rouse <[email protected]> Co-authored-by: Noémi Ványi <[email protected]> Co-authored-by: Manuel de la Peña <[email protected]> Co-authored-by: Chris Mark <[email protected]> Co-authored-by: Michael Madden <[email protected]> Co-authored-by: Shaunak Kashyap <[email protected]> Co-authored-by: Pablo Mercado <[email protected]> Co-authored-by: Ivan Fernandez Calvo <[email protected]> Co-authored-by: Mariana Dima <[email protected]> Co-authored-by: Jaime Soriano Pastor <[email protected]> Co-authored-by: Alex K. <[email protected]> Co-authored-by: Przemyslaw Gomulka <[email protected]> Co-authored-by: Amanda H. L. de Andrade Katz <[email protected]> Co-authored-by: Andrew Cholakian <[email protected]> * [Filebeat] Refactor mqtt input (elastic#16014) * Refactor mqtt input * Fix: comment * Add unit tests * Test: input run * Fix Test: run and stop * Test: backoff * Adjust code after review * MQTT: update docs (elastic#16152) * MQTT: add integration test (elastic#16143) * Create mosquitto image * MQTT input: add integration test * Fix * Verify connectivity * Fix * Fix: mage check * Fix * Fix * Fix: remove global var * Update changelog * Fix: regenerate notice file * Remove unused dependency * Fix: zero qos * Wait asynchronously for client being disconnected Co-authored-by: Felix <[email protected]> Co-authored-by: Adrian Serrano <[email protected]> Co-authored-by: DeDe Morton <[email protected]> Co-authored-by: Sophia Xu <[email protected]> Co-authored-by: Brandon Morelli <[email protected]> Co-authored-by: kaiyan-sheng <[email protected]> Co-authored-by: Blake Rouse <[email protected]> Co-authored-by: Noémi Ványi <[email protected]> Co-authored-by: Manuel de la Peña <[email protected]> Co-authored-by: Chris Mark <[email protected]> Co-authored-by: Michael Madden <[email protected]> Co-authored-by: Shaunak Kashyap <[email protected]> Co-authored-by: Pablo Mercado <[email protected]> Co-authored-by: Ivan Fernandez Calvo <[email protected]> Co-authored-by: Mariana Dima <[email protected]> Co-authored-by: Jaime Soriano Pastor <[email protected]> Co-authored-by: Alex K. <[email protected]> Co-authored-by: Przemyslaw Gomulka <[email protected]> Co-authored-by: Amanda H. L. de Andrade Katz <[email protected]> Co-authored-by: Andrew Cholakian <[email protected]>
leweafan
pushed a commit
to leweafan/beats
that referenced
this pull request
Apr 28, 2023
* [Filebeat] Fixes for NetFlow v9 devices from various vendors (elastic#15449) - Allow for zero scope fields in options template NetFlow v9 spec allows for options templates that contain no scope fields. The netflow input was treating this case as an error and discarding the template, but that is only applicable to IPFIX. - Use additional fields to populate bytes/pkt counters Some devices out there (Cisco NSEL) use fields 231/232 as bytes counters, when those are supposed to be layer 4 payload counters. This updates the ECS fields populator to use those fields when the expected ones are not found. - Support a classId of 32 bits While the spec mandates a classId of 8 bits, some Cisco ASA devices actually use a 32 bit version of this field. This patches the field to allow up to 32-bit integers and updates the index pattern to use `long` for the `netflow.class_id` field. - Add more fields from v9 Cisco devices Fixes elastic#14212 * update settings for `decode_csv_fields` (elastic#15249) (elastic#15550) Co-authored-by: DeDe Morton <[email protected]> Co-authored-by: Sophia Xu <[email protected]> * docs: updates to output config (elastic#15272) * [Filebeat] Handle error message in handleS3Objects function (elastic#15545) * Handle error message in handleS3Objects function * remove s3Context.Fail and use setError and done instead * Add changelog * Fix use of wrong fields in Cisco ASA dashboard (elastic#15553) This dashboard wasn't updated after a couple of fields were renamed. Fixes: elastic#15420 * Add test for publisher spool encode and decode. (elastic#15534) * Add test for publisher queue encode and decode. * Run mage fmt. * Fixes from code review. * Fix convert processor conversion of string with leading zeros to integer (elastic#15557) The conversion failed when for strings with leading zeroes and a decimal digit 8 or 9, as the underlying runtime function would try to parse that as an octal number. This is fixed by only allowing decimal and hex, which in turns makes the processor more aligned to its Elasticsearch counterpart. Fixes elastic#15513 * New mage target: generate pkg file to test the manager (elastic#15580) This PR adds a new mage target to Functionbeat named `buildPkgForFunction`. It generates the folder `pkg` with the functions to make testing the manager more comfortable during development. * Packetbeat TLS: Replace array fields with keyword (elastic#15597) Use of `type: array` in some fields (which was inconsistent) causes those fields to be excluded from the template. This prevents pointing aliases to those fields, which we need in 7.6+. Setting those fields to `keyword` explicitly so that they are included in the template. Fixes elastic#15588 * Add a pull request template providing valuable information when reviewing a PR (elastic#15388) * Add a PR template that provides valuable information when reviewing a PR * Add CLA check * Fix typo * Address comments during review * SF: Fix typo * Add deprecation as PR type * Make it clear how to strike through in markdown * Add default configuration files to the checklist * [Metricbeat] Implement IBM MQ module (elastic#15301) * Modify cockroachdb source * Define testdata * Do not publish ports * Update docs * mage fmt update * Describe containerized environment * Update CHANGELOG.next.asciidoc Co-Authored-By: Chris Mark <[email protected]> * Update data.json * Rename image * Update source after review * Filter ibmmq_ metrics * mage check * Fix: mage check * Don't expose port * Rename status to qmgr * Add subscriptions overview dashboard for IBM MQ module * Add calls, messages overview dashboard for IBM MQ module * Add screenshots * Fix: mage check * Fix: CHANGELOG * Add explanation * Fix: mage check Co-authored-by: Chris Mark <[email protected]> * Cleanup changelogs for master (elastic#15617) * Cleanup changelogs for master * Remove extra header in CHANGELOG.asciidoc * [Metricbeat] Add lambda metricset in aws module (elastic#15260) * Add lambda metricset * Adds missing imports (elastic#15624) * [docs] Clarify privileges required for the writer role (elastic#15604) * Mask password discovered via module autodiscover hint (elastic#15616) * Mask password is string representation of config * Rename method * Adding unit test * Use const for module config password setting name * Using common.DebugString * Simplifying * Removing now-invalid unit test * Removing now-unnecessary const * Refactoring: moving debug-related var and func to common file * Refactoring: rename from black list to mask list * Implement fmt.Formatter for common.MapStr * Reintroduce debug statement * Make MarshalLogObject always filter MapStr object for logging purposes * Refactoring: renaming to be bit more generic * Forgot to add license header to new file * Fixing verb syntax * Update KQL to get estimated cost without dimension ServiceName (elastic#15640) * Adding monitoring.cloud.* settings to reference files (elastic#15648) * Adding monitoring.cloud.* settings to reference files * Missed winlogbeat somehow * Missed x-pack/winlogbeat * remove lablels (elastic#15644) * Fix panic: don't send events if client is nil (elastic#15568) * Fix panic: don't send events if client is nil * Use mutex * Add CHANGELOG entry * Rename changelog entry * Fix: changelog * Temporarily use specific logstash release * [Metricbeat] Add Istio mesh metricset (elastic#15535) * [Metricbeat] Fix changelog (elastic#15681) * Fix changelog * ci: use APM pipeline library (elastic#15636) it uses APM pipeline library configured in the instance * AWS Lambda: downgrade Kibana dashboard (elastic#15682) * AWS Lambda: downgrade Kibana dashboard * Downgrade other AWS dashboards * Log command error if setup dashboards fails * Another downgrade * Use github.com/godror/godror instead of goracle.v2 (elastic#15683) From the README of goracle: > Goracle is deprecated because of naming (trademark) issues. From now on we are using github.com/godror/godror instead. * Move pdh query to shared location in order for new modules/metricsets to reuse (elastic#15503) * Move pdh query to shared location * Update changelog * Fix make update * mage fmt * fix changelog * Remove datasource option from SQL module and add tests (elastic#15686) Remove datasource option from SQL module. This option was intended to set the DSN of a database connection, and we were ignoring the hosts setting. In other SQL modules we are using the values in hosts as DSNs, do here the same for consistency. Host is redacted when we cannot parse it as it can contain passwords. StandardizeEvent is exposed in mbtest.Fetcher interface so we can more easily check contents of events in tests. Add integration tests of the module with MySQL and PostgreSQL. Add real data.json with data from MySQL and PostgreSQL. * [metricbeat] add service metricset to reference documentation (elastic#15643) * add service metricset to ref docs * update xpack docs * [metricbeat] Add divide by zero check to docker/diskio (elastic#15649) * add NaN check to docker/diskio * return 0 * Change joda style pattern to java style (elastic#15695) since 7.0 elasticsearch is using java.time style patterns. YYYY becomes yyyy * [DOCS] Add missing config options to shared file (elastic#15136) * [DOCS] Add missing config options to shared file * Add fixes from review * Run mage fmt update to fix build error * [Heartbeat] Support for multiple status codes elastic#13595 (elastic#15587) Allow for multiple status codes in config. Fixes elastic#13595 * Add missing changelog entry for elastic#15587 (elastic#15721) * Update github.com/godror/godror to v0.10.4 (elastic#15737) ## What does this PR do? This PR updates the dependency `github.com/godror/godror` to v0.10.4. ## Why is it important? Packaging of Metricbeat fails due to the issue in the `godror` version we are currently using. See more about the problem: godror/godror#8 * Collect normalized CPU percentages by default (elastic#15729) * Collect normalized CPU percentages by default * Adding CHANGELOG entry * Updating x-pack/metricbeat * Fix: mage check * Detect Eclipse Public License Co-authored-by: Adrian Serrano <[email protected]> Co-authored-by: DeDe Morton <[email protected]> Co-authored-by: Sophia Xu <[email protected]> Co-authored-by: Brandon Morelli <[email protected]> Co-authored-by: kaiyan-sheng <[email protected]> Co-authored-by: Blake Rouse <[email protected]> Co-authored-by: Noémi Ványi <[email protected]> Co-authored-by: Manuel de la Peña <[email protected]> Co-authored-by: Chris Mark <[email protected]> Co-authored-by: Michael Madden <[email protected]> Co-authored-by: Shaunak Kashyap <[email protected]> Co-authored-by: Pablo Mercado <[email protected]> Co-authored-by: Ivan Fernandez Calvo <[email protected]> Co-authored-by: Mariana Dima <[email protected]> Co-authored-by: Jaime Soriano Pastor <[email protected]> Co-authored-by: Alex K. <[email protected]> Co-authored-by: Przemyslaw Gomulka <[email protected]> Co-authored-by: Amanda H. L. de Andrade Katz <[email protected]> Co-authored-by: Andrew Cholakian <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type of change
What does this PR do?
This PR adds a GitHub template with sections that will improve the review process. There are a few sections that are mandatory:
and a few of them that are recommended, such as
Author's checklist,How to test this PR locally,Related issues,Use cases,Logs.Why is it important?
We want to achieve two main goals on behalf of achieving a very high software quality:
This PR description will serve as documentation not only for the reviewers, but for any occasional reader (technical writers, contributors, product managers, testers, customers).
The
Checklistsection is specially important, because it will serve a verification of the needed steps for a PR to be valid in terms of our development process. We (the team) should add here the elements we consider critical for the review process.Checklist
[ ] My code follows the style guidelines of this project[ ] I have commented my code, particularly in hard-to-understand areas[ ] I have added tests that prove my fix is effective or that my feature works[ ] New and existing unit tests pass locally with my changes[ ] New and existing integration tests pass locally with my changes[ ] Any dependent changes have been merged and published in downstream modules[ ] New dependencies have been vendorisedAuthor's Checklist
How to test this PR locally?
Doesn't apply
Related issues
Use cases
Doesn't apply
Screenshots
Doesn't apply
Logs
Doesn't apply
Mentions
cc/ @exekias @andresrc